ci: complete pre-commit.ci replacement and cache Lychee checks - #2993

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

EDIT: The links below are outdated, but the guidance in general is still useful.

I asked codex to guide me through reviewing this PR myself. Sharing what it gave me (in part because I want to click the links here myself):

Review Files changed against #2994; that isolates this layer. I’d use six passes, each with one question to answer before moving on.

1. When does it run, and what does it check?
   Start with lychee.yml’s triggers and checkout. Trace a PR update, nightly call, and manual run. There is no push trigger, so copy-pr-bot’s branch pushes do not add another run. Checkout selects the PR’s head SHA.

2. Are we selecting the intended documentation?
   Read the input selector, then its tests. Authored inputs are tracked Markdown/reST outside qa/; rendered inputs are HTML outside _static. Pay particular attention to skipping README symlinks: checking the same content from another directory can misinterpret relative links. Empty inputs must fail.

3. Does rendered checking use this PR’s sources?
   Follow the build helper. The important safeguards are verifying that all three libraries import from the checkout and match their installed metadata. Installing metapackage metadata with --no-deps preserves those local packages.

4. Does caching preserve the checking policy?
   Read cache preparation through publication. Trace both values of refresh-cache: PRs restore; nightly checks afresh and publishes. Version, policy, and authored/rendered kind separate the namespaces; successful URL checks expire after one day. Shared visibility relies on publishing from main, under GitHub’s cache scope rules.

5. Can failures accidentally produce a green check?
   Examine the checker and aggregate gate, then the nightly gate. Think through a failed docs build, one failed matrix leg, cancellation, and documentation-only mode. That mode requires successful link/tool checks and skipped wheel/GPU jobs.

6. Do the deletions have complete replacements?
   Finish with build-docs.yml, ci.yml, and the documentation changes. The dedicated workflow replaces inline lychee; [no-ci] ci: run pre-commit on Linux and Windows with Dependabot hook updates #2994 replaces Windows pre-commit. Live required-check migration belongs to ci: add required-check migration helper #3002.

/ok to test 27b3cf9

/ok to test ed21592

/ok to test d15a151

Converting to Draft mode before rebasing to fetch the latest updates from main. I don't want to trigger CI, only get a clean baseline for addressing @mdboom's feedback.

/ok to test 081ded4

@mdboom the CI is really slow today, but it looks like all jobs will be green eventually.

The below is mostly agent-generated (I switched up to codex GPT-6-Astra ultra), with small tweaks:

Replies to review: #2993 (review)

When CI monitoring stopped, full CI had 104 successful jobs and two A100 jobs queued, with no failures; its final status gate was still pending.

Canonical links for new documentation pages

You're right. Building the latest layout caused Sphinx to emit canonical URLs pointing at production, so a new page could fail with a 404 before it was published. The earlier successful CI runs didn't add a page and missed this case.

The original choice of BUILD_LATEST=1 was to assemble a consistent set of documentation trees for all four packages using the existing build scripts. We can retain that layout while checking the pages built for this revision: the source-build helper now sets CUDA_PYTHON_DOCS_DOMAIN to the absolute file:// URI of artifacts/docs. Canonical links resolve to the actual local output, so we check them rather than excluding them. This override is confined to the link-checking build; the wheel-based build and published canonical URLs keep their existing behavior.

Fixed in 456f145. A real build of all four documentation trees included two temporary new cuda-core pages with a link between them. All 1,118 canonical links resolved locally. With external network requests disabled, pinned Lychee passed both the new-page check and a complete local-file and anchor scan of the rendered tree. Removing the new target then correctly failed with a missing-file error. The committed regression tests also cover the helper's domain setup for paths with and without spaces. The actual PR documentation run additionally covered external links and passed both link jobs on their first checker attempt, with zero errors or timeouts.

Checking the PR merge revision

Agreed: the PR check should include its integration with the base branch. The original explicit head checkout was intended to give generated GitHub source links an ordinary commit SHA that GitHub could resolve. That precaution was unnecessary: we verified that GitHub source links at the test-merge SHA resolve and pass the pinned Lychee version.

The workflow now uses the default checkout revision, matching the standalone pre-commit workflow. For a PR event this is GitHub's test merge with the base branch; manual and nightly runs use their selected revision. Generated source links still use the actual checked-out SHA.

Fixed in 081ded4. A separate Git fixture reproduced the concern: the PR added a link while main removed its target, producing a clean merge. Pinned Lychee passed at the PR head and failed at the merge revision. In the actual PR run, both checkout logs confirm test-merge SHA 692e123, combining main at 4c893de with PR head 081ded4. All three jobs passed, including rendered documentation with source links at that merge SHA.

The nightly integration and cache reader also passed on their first workflow attempts. The reader restored both exact producer snapshots, passed both link checks, and skipped cache publication.

Full CI was triggered at PR head 081ded4 to cover the complete package build/test plan, the wheel-based documentation build and preview deployment, and the final status gate. The documentation build and preview deployment have both passed.

Retry count and the job timeout

The repeated passes reuse successful external-link results; only unsuccessful external checks and local files need checking again. We don't have enough representative cold-cache runs to estimate retry frequencies, and we have actually exhausted all ten attempts on a persistent HTTP 429, so the rationale shouldn't depend on that being extremely rare.

The two limits bounds are complementary safeguards: ten attempts limit repeated checking, while 60 minutes limits the total job duration.

Ten attempts means the initial check plus nine retries. The cooldowns total 17 minutes: one minute before the first retry, then two minutes before each of the remaining eight. That leaves 43 minutes for setup, building the documentation, and all ten checking passes.

Our measurements suggest substantial room within that budget. Building the documentation took about seven minutes, and a cold rendered check took about five minutes. Subsequent checks reuse successful external results: retries during an observed persistent HTTP 429 took roughly 40-60 seconds each.

Using those timings, even nine retries would give roughly 7 + 5 + 9 + 17 = 38 minutes, plus setup overhead. Consistent with that estimate, an actual job exhausted all ten attempts in about 37 minutes.

Codex — 2026-10-08T17:05:21-07:00 (PDT) · +2m 7s after Codex · +1h 24m since User

Implemented and pushed to PR 2993.

• Three-pass retry limit; bounded 429 warnings; advisory nightly link failures with checker errors remaining fatal.
• Repaired moved sample links and nightly documentation setup.
• Local hooks, 220 tests plus 36 subtests, nightly producer, and cache reader passed.
• Full CI passed all 107 jobs without reruns.

The description now includes the policy as its first expandable section and current validation evidence.

The worktree is clean, PR branches are synchronized, and 2993 remains Draft. I rebased immediately before pushing; main advanced while CI ran, which the description records.

Fable 5 re-review:

Verdict: the new retry and policy logic is correct as far as I can trace it, and the three commits since my last pass resolve most of what I raised. Nothing blocks merging on the PR's side. The two red checks are still main's stale lockfile, not this PR. The added-work answer is unchanged: no per-PR docs build, one new nightly source build.

What changed since the last review

The head moved from 585450e to 54ee0bb with three commits. The description now matches the code, including the validated head.

• Retry bound dropped from 10 passes to 3, with 60 s and 120 s cooldowns. Each check now spends at most three minutes in outer cooldowns instead of up to 17.
• Two failure policies. Full CI uses check: pass, or pass with a warning when only HTTP 429s remain on at most 10 distinct URLs, otherwise fail. Nightly uses cache-warm: any valid link failure is a warning and the cache is still published. Machinery failures stay hard failures under both.
• Nightly builds with PIXI_FROZEN instead of PIXI_LOCKED, so a stale workspace lockfile no longer blocks the nightly rendered job.
• Seven sample READMEs point at moved pages. I probed all seven new targets and they return 200. All five old targets I probed return 404, and the Nsight Compute anchor exists on the page.
• Earlier nits fixed: the duplicate GITHUB_REF export, the nightly header comment, and the ambient-Python dependency, which is now documented at .github/actions/check-doc-links/action.yml:19.

Findings

1. Nightly link rot is now invisible by default. Under cache-warm, a 404 in main's docs is a warning in a nightly job summary that nobody reads, and the next unrelated PR's full CI is what fails. That is the intended trade for keeping the nightly suite green, but it means the first person to hit a dead link is a random contributor. A cheap follow-up would be a nightly step that opens or updates a single tracking issue when the final report has permanent failures. ci/tools/retry_lychee.py:159

2. A permanent failure in nightly stops the 429 retries too. When a 404 and some 429s coexist, the loop exits on the first pass, warns, and publishes the cache. The rate-limited URLs never get their second and third passes, so the cache stays colder than it could be for that day. The description acknowledges this. If it matters, the cache-warm policy could keep retrying while any transient failure remains. ci/tools/retry_lychee.py:197

3. PIXI_FROZEN trades one failure mode for another. Frozen installs the committed lock without checking it against the manifest. If someone adds a docs dependency to the cuda_core manifest without relocking, the nightly builds against the old environment and fails or misbehaves inside Sphinx rather than at a clear lock check. The repo's separate lock-freshness check catches the drift, so this is acceptable. Worth knowing when reading a confusing nightly build log. .github/workflows/lychee.yml:42

4. Still true from the last review: network-level failures without an HTTP code, such as connection resets and DNS errors, are never retried by the outer loop. Lychee's own three per-request retries remain the only cover. ci/tools/retry_lychee.py:47

5. Still true: whether a top-level concurrency block applies to a called workflow is undocumented. Harmless because the nightly caller has its own group. .github/workflows/lychee.yml:32

6. Still true: the red checks on this head come from main's stale cuda_core lockfile. The PR's validated base is the same commit that fails on main. Merge is safe since the nightly no longer depends on the lock check.

7. Nit: the retry passes still add --mode task, which the first pass through lychee-action does not use. Harmless with JSON output.

8. Nit: the pre-commit.ci block in .pre-commit-config.yaml:5 stays until the App is detached, as the description says.

Added work vs. before PR 2994

Unchanged from my earlier answer. Nothing new runs on a PR push. Full CI loses the Windows pre-commit job and gains a cheap authored lychee pass inside the existing Docs job. The nightly gains two ubuntu jobs, one of which builds docs from source once a day. The shorter retry bound makes the worst case cheaper than before: a fully rate-limited check now costs about three minutes of cooldown plus three passes instead of seventeen minutes plus ten passes.

Verified

• All 220 CI-tool tests and 36 subtests pass locally with the nightly's invocation. The pre-commit hooks that apply to the changed files pass, including actionlint and ruff.
• Traced every path through the retry loop: exit codes 1 and 3 stop without retry under both policies; mixed 404 plus 429 fails check on pass one; all-429 reaches the allowance only after three passes; the distinct-URL count deduplicates across source files; argparse rejects unknown policies as a machinery failure.
• The build-docs caller passes no policy and gets check. The lychee workflow maps refresh-cache to cache-warm, so the nightly and a manual refresh warm, while a manual reader run enforces.
• The new tests cover the 10 versus 11 boundary, cross-source deduplication, the seven non-429 shapes the allowance must reject, cache-warm warnings for permanent failures, and the CLI defaults. All carry the agent-authored marker.

Responses to the PR-specific findings in the Fable 5 re-review, for 54ee0bb:

• 1. Nightly visibility — intentional tradeoff; possible follow-on. The failures remain visible as warning annotations, job summaries, and report artifacts, but agreed that those can go unnoticed. Nightly did not previously check these links, so this adds advisory detection. An unrelated PR can still be the first run to enforce a permanent failure. Automatically maintaining a tracking issue could be useful, but I would keep that notification behavior outside this PR.

• 2. Mixed permanent/transient failures — keep the current policy. Correct: a 404 alongside 429s stops further passes, leaving a less complete cache. That is acceptable for the present goal of producing a useful partial cache. Successful checks are retained; unresolved URLs are not cached as successes. Continuing retries specifically for cache warming is a possible later optimization, with the same three-pass ceiling.

• 3. PIXI_FROZEN — accepted tradeoff. Correct: frozen mode uses the committed documentation environment without verifying agreement with the manifest. Adding a documentation dependency still requires updating the lockfile. Build and checker failures remain fatal; frozen mode does not turn them into warnings. The fresh source documentation build was validated successfully on GitHub runners.

• 4. Network errors — two clarifications. HTTP(S) timeouts are eligible for outer retries even without an HTTP status code. Also, Lychee’s native retry setting does not guarantee retries for every network error: its pinned implementation excludes non-timeout connection errors and recognizes some connection resets as retryable. I would retain the narrow outer classification here and expand it only against concrete failure examples and tests. See the outer classifier and native classifier.

• 5. Concurrency — no change proposed. The nightly caller has its own concurrency control, so nightly scheduling does not depend on resolving the question about the called workflow’s top-level block. The link workflow’s block also controls direct manual dispatches. Caller and callee groups are deliberately distinct, and documentation-only tests are separated from full nightly runs.

• 7. --mode task — already consistent. The initial pass uses task mode too. The pinned action defaults checkbox to true, which its entrypoint translates into --mode task. The retry helper supplies the same option explicitly. There is no mode difference to fix. See the default and initial invocation.

• 8. pre-commit.ci configuration — intentionally retained. I want to minimize churn during the cutover. I’ll coordinate the Ruleset changes and App access removal with a repo admin shortly after merge. Removing the unused configuration can follow separately.

One clarification on added work: ordinary PR updates run the replacement Linux/Windows pre-commit jobs, but add no documentation build or Lychee pass. Full CI adds authored-link checking after its existing documentation build. Its cost depends on the cache: the measured authored Lychee execution was about 152 seconds cold versus 0.022 seconds warm, excluding setup. Nightly adds the fresh source build and cache production.

I'm a little concern about adding about ~9min of compute time to every PR

@mdboom This is fixed now; I also realized that my original, simple 10-retry idea doesn't work as intended when there are servers with long rate limits, so I changed the retry logic completely. This is documented in detail in the updated PR description.

Converting back to draft, until the pixi freshness situation is more stable (see #3065).